Skip to content

Add bitwarden-test-toolkit plugin with assessing-test-coverage skill#169

Open
nthompson-bitwarden wants to merge 11 commits into
mainfrom
test-engineer-plugin-current-coverage
Open

Add bitwarden-test-toolkit plugin with assessing-test-coverage skill#169
nthompson-bitwarden wants to merge 11 commits into
mainfrom
test-engineer-plugin-current-coverage

Conversation

@nthompson-bitwarden

Copy link
Copy Markdown

🎟️ Tracking

https://bitwarden.atlassian.net/browse/QA-1983

📔 Objective

Add the bitwarden-test-toolkit plugin to the marketplace, providing an assessing-test-coverage skill that analyzes a repository's existing test stack and reports coverage gaps — with supporting references and evals, plus the required marketplace registration and dictionary/README updates.

@nthompson-bitwarden
nthompson-bitwarden requested a review from a team as a code owner July 22, 2026 16:05
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

Plugin Validation Summary — bitwarden-test-toolkit

PR #169 · Validated the changed files in plugins/bitwarden-test-toolkit.

Result: ✅ All checks pass. No critical or major issues. A few minor/advisory notes are listed below — none block merge.

Validations performed:

  1. Plugin structure (plugin-validator agent) — manifest, directory layout, frontmatter, consistency, credentials
  2. Skill review (skill-reviewer agent) — assessing-test-coverage/SKILL.md
  3. Security review (reviewing-claude-config skill) — secrets, credentials, permission scoping

1. Plugin Structure — ✅ PASS

Check Result
plugin.json valid JSON, kebab-case name, semver 1.0.0, required fields
Directory layout & skill auto-discovery (skills/<name>/SKILL.md)
Name/version consistency across plugin.json, marketplace.json, CHANGELOG.md ✅ all agree at bitwarden-test-toolkit / 1.0.0
marketplace.json description matches plugin.json exactly
README.md present & comprehensive; CHANGELOG.md in Keep a Changelog format
No stray files (.DS_Store, __pycache__, result.json)
No hardcoded credentials
evals/ harness (Python runner + JSON fixtures) placement & structure ✅ appropriately co-located with the skill

No components other than the one skill (no agents, commands, hooks, or MCP servers) — as expected.

2. Skill Review — assessing-test-coverage/SKILL.md — ✅ PASS

  • Frontmatter: valid; name matches directory; description present.
  • Description quality: exemplary — quotes 5 positive trigger phrases and enumerates 5 explicit out-of-scope counter-examples, giving strong collision avoidance against strategy/design/test-authoring requests.
  • Writing style: imperative/infinitive throughout; clear numbered Steps → Gotchas → Output template flow.
  • Word count: ~615 words — below the 1,000–3,000 guideline, but appropriate and deliberate for a linear procedural workflow with a fixed output template. Padding would dilute it.
  • Progressive disclosure: core SKILL.md is lean; the output template is inlined (single load-bearing artifact); evals/ is dev/CI tooling correctly kept off the runtime content path.
  • Reference integrity: all references resolve — Skill(bitwarden-atlassian-tools:researching-jira-issues) exists; ${CLAUDE_PLUGIN_DATA} usage is consistent with the README.
  • Notable strengths: treats Jira/Confluence/PR/CSV input as untrusted data and cites CWE-1427 (prompt injection); least-privilege sub-command-scoped Bash grants; anti-hallucination guardrail ("never assert 'no tests' for a surface you have not inspected").

3. Security Review — ✅ PASS (no findings)

  • Committed secrets / credentials: none. The only token occurrence is TARGET_SKILL_TOKEN = "assessing-test-coverage" (evals/run_real_eval.py:22) — a skill-name identifier, not a secret.
  • Settings files: none in this PR — no settings.local.json risk.
  • Permission scoping (SKILL.md:5): Bash grants are tightly scoped to specific sub-commands (Bash(gh pr view:*), Bash(date:*), etc.) rather than blanket Bash(*) — good least-privilege. No destructive or auto-approved dangerous commands.
  • JSON fixtures: contain only natural-language test queries and public repo/PR references — no sensitive data.
  • run_real_eval.py: developer/CI eval tooling only; passes inherited env minus CLAUDECODE to claude -p subprocesses; never loaded at skill runtime, no exfiltration.

Minor / Advisory Notes (non-blocking)

  1. SKILL.md:4argument-hint field: this is a slash-command frontmatter field, not a documented skill field. It is inert (ignored) for a skill since skills trigger on natural language. Harmless as self-documentation; optionally remove or fold the input formats into the description/body.
  2. SKILL.md:27 / README.md:13${CLAUDE_PLUGIN_DATA}: referenced but not defined within the plugin (unlike the standard ${CLAUDE_PLUGIN_ROOT}). Confirm the host runtime provides this variable; otherwise the report path resolves to a bare relative coverage-reports/…. Used consistently in both files, so this appears intentional.
  3. evals/run_real_eval.py:135 vs evals/README.md:13--model default: the runner defaults to claude-opus-4-7, while the recorded baseline used claude-sonnet-4-6. Intentional (the README example overrides the default), but aligning the default or adding a one-line note would prevent confusion when reproducing the baseline.

Recommendation: APPROVE. All structural, skill-quality, and security checks pass. The three notes above are advisory and do not require changes before merge.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the addition of the bitwarden-test-toolkit plugin and its assessing-test-coverage skill, including the marketplace registration, plugin manifest, README, changelog, cspell entry, and the Python trigger-eval harness. Verified the required version/changelog/marketplace/README/cspell updates are all present and internally consistent (name, version 1.0.0, and description match across marketplace.json, plugin.json, and the README table). The SKILL.md frontmatter is valid, well-scoped, and treats external input (Jira/PR/CSV) as untrusted data (CWE-1427). The earlier-flagged README threads (workflow diagram referencing nonexistent files, permalink guidance contradiction) are no longer present in the current tree.

Code Review Details

No blocking or actionable findings.

Notes considered and intentionally not flagged:

  • ${CLAUDE_PLUGIN_DATA} is an established convention already used by bitwarden-code-review, so its use here is consistent.
  • The run_real_eval.py harness is a dev/eval utility; its stream-parsing, timeout, and early-bail logic are correct for that purpose. The --model default differs from the recorded baseline model, which is documented in the evals README.
  • Bash(git clone:*) in allowed-tools is broad but scoped to the skill's stated purpose and gated behind explicit user consent per the SKILL.md gotchas.

Comment thread .gitignore Outdated
nthompson-bitwarden and others added 7 commits July 22, 2026 12:11
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Inline the previous references/ content into a lean, self-contained
SKILL.md step list and delete the reference files. Tighten the skill
description with explicit scope exclusions to sharpen triggering, add a
prompt-injection guard for untrusted input (CWE-1427), and allow
git clone so uncloned repos can be inspected before being marked
unverified.

Rework the trigger eval with a portable runner that watches for the
real installed skill, refresh the README, and re-record the baseline.
Latest run: 10/10 should-trigger, 10/10 should-not-trigger.
@nthompson-bitwarden nthompson-bitwarden added the ai-review Request a Claude code review label Jul 24, 2026
Comment thread plugins/bitwarden-test-toolkit/README.md Outdated
Comment thread plugins/bitwarden-test-toolkit/README.md Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant